The Static Analysis tool Coverity added that |rv| is assigned a value that afterwards in never used: >> if (aFileManager->EnforcingQuota()) { >> rv = file->GetFileSize(&fileSize); >> if (NS_WARN_IF(!file)) { >> return NS_ERROR_FAILURE; >> } >> } I consider that in case of GetFileSize fails the correct behavior would be: >> if (aFileManager->EnforcingQuota()) { >> rv = file->GetFileSize(&fileSize); >> if (NS_WARN_IF(NS_FAILED(rv))) { >> return rv; >> } >> } In case GetFileSize gets called we are safe to assume that file is a valid nsCOMPtr<nsIFile> because of the earlier checker from the start of the function: >> nsCOMPtr<nsIFile> file = aFileManager->GetFileForId(mDirectory, aId); >> if (NS_WARN_IF(!file)) { >> return NS_ERROR_FAILURE; >> } so checking again the validity of file is useless.

Yes, you are right.

Review commit: https://reviewboard.mozilla.org/r/41391/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/41391/

that was wicked fast Jan :)

Comment on attachment 8732828 [details] MozReview Request: Bug 1258339 - check return value of GetFileSize. r?janv https://reviewboard.mozilla.org/r/41391/#review37863 r=me, Thanks!